Skip to content

fix: log audit log failures instead of dropping them#1769

Open
AmanGIT07 wants to merge 3 commits into
mainfrom
fix/audit-log-errors
Open

fix: log audit log failures instead of dropping them#1769
AmanGIT07 wants to merge 3 commits into
mainfrom
fix/audit-log-errors

Conversation

@AmanGIT07

@AmanGIT07 AmanGIT07 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Legacy audit log writes (audit.Logger.Log / LogWithAttrs) discarded their error in the connect handlers and in three core services. Failures are now logged. The calling operation still succeeds, matching the existing handling in CreateOrganization.

Changes

  • Connect handlers (12 files): check the audit log error and record it through the existing ErrorLogger with operation tag <RPC>.AuditLog
  • core/membership: log audit log failures through the service's slog logger
  • core/deleter, core/organization: log audit log failures through slog.WarnContext
  • Package helpers with no request in scope (logAuditForCheck, auditPolicyCreationEvent) log through slog.ErrorContext

Technical Details

No caller-visible behavior change: an audit log failure never fails the request.

Test Plan

  • go test ./internal/api/v1beta1connect/ ./core/membership/ ./core/deleter/ ./core/organization/ passes
  • Build and type checking passes

AmanGIT07 and others added 2 commits July 17, 2026 14:36
@AmanGIT07 @claude
fix(connect): log audit failures instead of dropping them
cc91053 
Audit log writes in the connect handlers discarded their error. Check
the error and record it through the existing error logger. Requests
still succeed when the audit write fails.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AmanGIT07 @claude
fix(core): log audit log failures instead of dropping them
65c65f1 
Legacy audit log writes in the membership, deleter, and organization
services discarded their error. Log a warning when the write fails.
The calling operation still succeeds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment Jul 17, 2026 9:33am

@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@AmanGIT07, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b46c87bd-8e3a-4335-a33e-b9b5123b706e

📥 Commits

Reviewing files that changed from the base of the PR and between 65c65f1 and 0a4c843.

📒 Files selected for processing (1)
  • internal/api/v1beta1connect/kyc_test.go
📝 Walkthrough

Walkthrough

Audit logging across core services and Connect API handlers now checks returned errors and emits structured warnings or service-error logs. Primary organization, membership, policy, project, role, service-user, and user operations retain their existing control flow and responses.

Changes

Audit logging error handling

Layer / File(s) Summary
Core service audit reporting
core/deleter/service.go, core/membership/service.go, core/organization/service.go		 Organization and membership audit writes now warn when logging fails.
Organization and billing handler reporting
internal/api/v1beta1connect/authorize.go, billing_customer.go, group.go, kyc.go, organization.go		 Handlers now report audit write failures through service error logging with relevant identifiers.
Permission and policy audit reporting
internal/api/v1beta1connect/permission_check.go, policy.go		 Permission and policy audit failures now produce structured diagnostics.
Project and resource audit reporting
internal/api/v1beta1connect/project.go, resource.go		 Project and resource handlers now capture audit failures without changing normal responses.
Role and user audit reporting
internal/api/v1beta1connect/role.go, serviceuser.go, user.go		 Role, service-user, and user handlers now check audit writes and log failures with operation-specific context.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: rohilsurana

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coveralls

coveralls commented Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage Report for CI Build 29570336713

Coverage decreased (-0.1%) to 45.975%

Details

  • Coverage decreased (-0.1%) from the base build.
  • Patch coverage: 132 uncovered changes across 14 files (56 of 188 lines covered, 29.79%).
  • 7 coverage regressions across 3 files.

Uncovered Changes

Top 10 Files by Coverage Impact Changed Covered %
internal/api/v1beta1connect/role.go 28 2 7.14%
internal/api/v1beta1connect/project.go 20 2 10.0%
internal/api/v1beta1connect/user.go 24 9 37.5%
core/membership/service.go 24 12 50.0%
internal/api/v1beta1connect/organization.go 11 2 18.18%
internal/api/v1beta1connect/resource.go 15 6 40.0%
internal/api/v1beta1connect/group.go 12 4 33.33%
internal/api/v1beta1connect/policy.go 12 4 33.33%
internal/api/v1beta1connect/serviceuser.go 12 4 33.33%
internal/api/v1beta1connect/authorize.go 6 0 0.0%
Total (15 files) 188 56 29.79%

Coverage Regressions

7 previously-covered lines in 3 files lost coverage.

File Lines Losing Coverage Coverage
internal/api/v1beta1connect/role.go 4 26.92%
internal/api/v1beta1connect/project.go 2 44.0%
internal/api/v1beta1connect/organization.go 1 63.27%

Coverage Stats

Coverage Status
Relevant Lines: 38438
Covered Lines: 17672
Line Coverage: 45.98%
Coverage Strength: 13.29 hits per line
💛 - Coveralls

@AmanGIT07 @claude
test(connect): assert audit log failure does not fail the request
0a4c843 
Inject a failing audit repository into the handler context and verify
SetOrganizationKyc still succeeds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@AmanGIT07
AmanGIT07 requested a review from rohilsurana July 17, 2026 10:17
@coderabbitai coderabbitai Bot mentioned this pull request Jul 17, 2026
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rohilsurana rohilsurana Awaiting requested review from rohilsurana

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AmanGIT07 @coveralls